-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: added Unity build tasks to BuildTasksContainer #31
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good, but has to be reformated a bit
namespace StansAssets.Build.Pipeline | ||
{ | ||
class DefaultBuildTasksProvider : IBuildTasksProvider | ||
{ | ||
public IBuildTasksContainer GetBuildSteps(IUserEditorBuildSettings buildSettings) | ||
public IBuildTasksContainerFull GetBuildSteps(IUserEditorBuildSettings buildSettings) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IBuildTasksProvider
Should only provide steps for the current build system.
IBuildTasksContainerFull
- doesn't make much scene in terms of API.
We want to only display those additional default Unity steps in the editor window. No need to change the tool API.
public interface IBuildTasksContainerFull : IBuildTasksContainer | ||
{ | ||
/// <summary> | ||
/// Pre process build steps. Order matters. | ||
/// </summary> | ||
IReadOnlyList<IOrderedCallback> UnityPreBuildTasks { get; } | ||
|
||
/// <summary> | ||
/// Post process build steps. Order matters. | ||
/// </summary> | ||
IReadOnlyList<IOrderedCallback> UnityPostBuildTasks { get; } | ||
|
||
/// <summary> | ||
/// Scene process build steps. Order matters. | ||
/// </summary> | ||
IReadOnlyList<IOrderedCallback> UnityProcessSceneTasks { get; } | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again we don't really need this to be a part of our API
6edec44
to
7f0f9f3
Compare
This PR depends on the custom branch of the com.stansassets.foundation package. |
Extend the Unity related build steps visualization:
I have also
|
@@ -0,0 +1,7 @@ | |||
namespace StansAssets.Build.Pipeline | |||
{ | |||
public interface IBuildStepsViewModelProvider |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…ity build tasks: IPreprocessBuildWithReport, IPostprocessBuildWithReport, IProcessSceneWithReport
- collecting of Unity built-in tasks is moved to BuildPipelineTab
…ps in-editor visualization
9cc52a3
to
d8d3e8c
Compare
Purpose of this PR
BuildTaskContainer now contains user's Unity build tasks too.
If user's tasks implement interfaces IPreprocessBuildWithReport, IPostprocessBuildWithReport, IProcessSceneWithReport it will be added to the list
Testing status
Manual testing status
Tested manually in Unity Editor